-
Notifications
You must be signed in to change notification settings - Fork 12.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix clobber_abi and disallow SVE-related registers in Arm64EC inline assembly #131332
base: master
Are you sure you want to change the base?
Conversation
compiler/rustc_target/src/asm/mod.rs
Outdated
p0, p1, p2, p3, p4, p5, p6, p7, | ||
p8, p9, p10, p11, p12, p13, p14, p15, | ||
ffr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given the reason why v16-v31 are blocked in this target, I suspect that SVE-related registers (z*
, p*
, ffr
) may actually be unavailable in Arm64EC.
Although AFAIK they have not documented anything about it in https://learn.microsoft.com/en-us/windows/arm/arm64ec-abi#register-mapping-and-blocked-registers
(I don't think this question is a blocker for this PR, but I think it could be a blocker for stabilization.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pmsjt can we get the docs updated to indicate if SVE-related registers (z*
, p*
, ffr
) are available or not on ARM64EC?
compiler/rustc_target/src/asm/mod.rs
Outdated
p0, p1, p2, p3, p4, p5, p6, p7, | ||
p8, p9, p10, p11, p12, p13, p14, p15, | ||
ffr, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pmsjt can we get the docs updated to indicate if SVE-related registers (z*
, p*
, ffr
) are available or not on ARM64EC?
@dpaoliello SVE use in Windows hasn't been documented in either Arm64 classic or Arm64EC. That is coming soon and, when it does, it'll cover both. To answer your most immediate question: No, SVE should not be used by Arm64EC code. No Arm64EC code should ever access SVE state or ISA regardless of the CPU supporting it. Just as for the presently reserved GP and Neon state in EC [1], SVE will also be reserved and not mapped to any x86-64 state. [1] https://learn.microsoft.com/en-us/cpp/build/arm64ec-windows-abi-conventions |
08eb832
to
ed6c4a6
Compare
This comment has been minimized.
This comment has been minimized.
a0f78ea
to
3307178
Compare
Some changes occurred in compiler/rustc_codegen_gcc |
3307178
to
ff07448
Compare
@pmsjt Thanks for the clarification! I updated PR to disallow SVE-related registers. |
This comment has been minimized.
This comment has been minimized.
ff07448
to
df0358e
Compare
This comment has been minimized.
This comment has been minimized.
df0358e
to
798e683
Compare
This comment has been minimized.
This comment has been minimized.
d380cfd
to
5706a48
Compare
a3ceff4
to
aded791
Compare
aded791
to
a4fd8b1
Compare
It's not necessary to disallow the use of the |
a4fd8b1
to
273efe4
Compare
I see. I removed z0-z15 related changes and added note about SVE vector types. rust/compiler/rustc_target/src/asm/aarch64.rs Lines 64 to 67 in d858dfe
|
273efe4
to
d858dfe
Compare
I also removed AArch64InlineAsmReg::emit-related change since it is no longer related to this PR. I will open a separate PR for that. UPDATE: opened #131667 |
Currently
clobber_abi
in Arm64EC inline assembly is implemented usingInlineAsmClobberAbi::AArch64NoX18
, but broken since it attempts to clobber registers that cannot be used in Arm64EC: https://godbolt.org/z/r3PTrGz5rAdditionally, this disallows SVE-related registers per #131332 (comment).
cc @dpaoliello
r? @Amanieu
@rustbot label O-windows O-AArch64 +A-inline-assembly